fix(daemon): move the self-test probe out of the shared /tmp namespace - #2
Merged
Conversation
cordond is uid 0 and the self-test runs *before* seed_policy, so cordon is not yet enforcing on itself. The probe was $TMPDIR/cordon-selftest.<pid>/, built with create_dir_all (which accepts a pre-existing directory of any owner) and seeded with fs::write (O_WRONLY|O_CREAT|O_TRUNC, no O_NOFOLLOW). A local user who pre-creates that directory holding `src -> <any root-owned file>` gets root to truncate it on the next start; pids are sequential, so guessing is cheap. Aimed at /etc/cordon/policy.toml it is a boundary kill: the daemon then fails to compile the policy and enforces nothing. fs.protected_symlinks does not cover this. may_follow_link() keys on the symlink's immediate parent being sticky and world-writable, and there that parent is the attacker's own plain directory, so the check returns before comparing uids. The probe moves to /run/cordon, which is root-only because /run is root-owned 0755 — that removes the shared namespace outright instead of hardening each open against it. Secondarily the probe is now removed and recreated with create_dir rather than reused wherever it is found, so a found directory (and any symlink in it) is gone before the seed writes. That guard races on its own and is not the fix; it keeps the failure visible if the location ever moves back. Regression tests, both verified to fail without their half of the fix: the planted-directory test reproduces the truncation (victim reads "x"), and a constant check pins the probe under /run, which an unprivileged test cannot exercise directly. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Item 1 of the resequenced queue (see #1): the one carried-over note that is a live escalation rather than a rough edge.
The bug
cordondis uid 0 and the self-test runs beforeseed_policy, so cordon is not yet enforcing on itself. The probe was$TMPDIR/cordon-selftest.<pid>/, built withcreate_dir_all— which accepts a pre-existing directory of any owner — and seeded withfs::write, which isO_WRONLY|O_CREAT|O_TRUNCwith noO_NOFOLLOW.A local user pre-creates that directory holding
src -> <any root-owned file>and root truncates it on the next start. Pids are sequential, so guessing is cheap. Aimed at/etc/cordon/policy.tomlit is a boundary kill: the daemon then fails to compile the policy and enforces nothing.fs.protected_symlinksdoes not cover this —may_follow_link()keys on the symlink's immediate parent being sticky and world-writable, and there that parent is the attacker's own plain directory, so the check returns before comparing uids.The fix
The probe moves to
/run/cordon, root-only because/runis root-owned0755. That removes the shared namespace outright rather than hardening each open against it — noO_NOFOLLOW/O_EXCLdance needed, because nothing unprivileged can plant any component of the path.Secondarily, the probe is removed and recreated with
create_dirinstead of being reused wherever found, so a found directory and any symlink in it are gone before the seed writes. That guard races on its own and is not the fix — it keeps the failure visible if the location ever moves back.probe_dir()becamemake_probe(base), absorbing the seed loop, so the step that actually follows the symlink is reachable from an unprivileged test.Tests
Both verified to fail without their half of the fix, not merely asserted to:
the_probe_never_reuses_a_directory_it_found— plantsselftest.<pid>/src -> policy.toml, asserts the victim survives. Reverting tocreate_dir_allreproduces the truncation:left: "x",right: "the live policy".the_probe_lives_where_only_root_can_write— pinsPROBE_DIRunder/run/. Weaker, but it covers the half an unprivileged test cannot reach; flipping the constant to/tmp/cordonfails it.Verification
cargo test131 passed / 0 failed.run-bpf-lsm-vm.shgreen on every scenario on the pinned v6.12 — load-bearing here, since the self-test is fail-closed and the daemon refuses to start if the probe path breaks.Not done here: retiring the "still live" note for this bug in
PLAN-rule-model.md, because #1 edits the same bullet. Follow-up once that merges.🤖 Generated with Claude Code